Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Misplaced Screen Title & Fix Layout Structure #2412

Merged
merged 32 commits into from
Nov 8, 2024

Conversation

VanshikaSabharwal
Copy link
Contributor

@VanshikaSabharwal VanshikaSabharwal commented Nov 7, 2024

What kind of change does this PR introduce?

bugfix

Issue Number:

Fixes #2071

Did you add tests for your changes?

No

Snapshots/Videos:

Before
image
After
image

If relevant, did you update the documentation?

Summary

I updated the code such that the "Posts" title aligns properly.

Does this PR introduce a breaking change?

No

Other information

Have you read the contributing guide?

Yes

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced type safety in chat-related components.
    • Improved test coverage for the OrganizationCard and UserScreen components.
  • Bug Fixes

    • Streamlined title rendering in the UserScreen component.
  • Documentation

    • Updated test descriptions for clarity in the UserScreen tests.
  • Chores

    • Removed unused translation imports across various components and tests.

Copy link
Contributor

coderabbitai bot commented Nov 7, 2024

Walkthrough

The changes in this pull request primarily involve the removal of the useTranslation hook from several components and their corresponding test files, indicating a shift away from direct translation functionalities. Additionally, type safety has been enhanced by updating function signatures to use more specific types. The tests have been refined for clarity and improved coverage, particularly for the OrganizationCard and UserScreen components, while maintaining the overall structure and functionality of the components.

Changes

File Change Summary
src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx Removed useTranslation import; tests remain unchanged in logic.
src/components/UserPortal/CreateDirectChat/CreateDirectChat.tsx Updated id type in chatsListRefetch from any to string; removed useTranslation hook.
src/components/UserPortal/CreateGroupChat/CreateGroupChat.tsx Updated return type of chatsListRefetch to Promise<ApolloQueryResult<string>>; removed useTranslation import.
src/components/UserPortal/OrganizationCard/OrganizationCard.test.tsx Consolidated imports; tests verify rendering and functionality of OrganizationCard.
src/components/UserPortal/UserSidebar/UserSidebar.test.tsx Removed waitFor import; retained asynchronous handling with wait.
src/screens/UserPortal/Chat/Chat.test.tsx Renamed test cases for clarity; no changes to logic.
src/screens/UserPortal/Posts/Posts.tsx Removed InterfaceAdContent and AdvertisementsConnection; updated InterfacePostComments.
src/screens/UserPortal/UserScreen/UserScreen.test.tsx Added useNavigate mock; updated test descriptions for clarity.
src/screens/UserPortal/UserScreen/UserScreen.tsx Simplified title rendering logic; no changes to exported entities.

Assessment against linked issues

Objective Addressed Explanation
Move screen titles to parent outlet container (#2071) The changes do not address the screen title placement issue.

Possibly related issues

Possibly related PRs

Suggested labels

no-pr-activity

Suggested reviewers

  • pranshugupta54
  • DMills27

Poem

In the portal where chats do play,
The translations have hopped away.
With types more clear, the tests now shine,
A cleaner code, oh how divine!
Let's chat and group, with style and grace,
In our User Portal, a joyful space! 🐇✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

github-actions bot commented Nov 7, 2024

Our Pull Request Approval Process

We have these basic policies to make the approval process smoother for our volunteer team.

Testing Your Code

Please make sure your code passes all tests. Our test code coverage system will fail if these conditions occur:

  1. The overall code coverage drops below the target threshold of the repository
  2. Any file in the pull request has code coverage levels below the repository threshold
  3. Merge conflicts

The process helps maintain the overall reliability of the code base and is a prerequisite for getting your PR approved. Assigned reviewers regularly review the PR queue and tend to focus on PRs that are passing.

Reviewers

Do not assign reviewers. Our Queue Monitors will review your PR and assign them.
When your PR has been assigned reviewers contact them to get your code reviewed and approved via:

  1. comments in this PR or
  2. our slack channel

Reviewing Your Code

Your reviewer(s) will have the following roles:

  1. arbitrators of future discussions with other contributors about the validity of your changes
  2. point of contact for evaluating the validity of your work
  3. person who verifies matching issues by others that should be closed.
  4. person who gives general guidance in fixing your tests

CONTRIBUTING.md

Read our CONTRIBUTING.md file. Most importantly:

  1. PRs with issues not assigned to you will be closed by the reviewer
  2. Fix the first comment in the PR so that each issue listed automatically closes

Other

  1. 🎯 Please be considerate of our volunteers' time. Contacting the person who assigned the reviewers is not advised unless they ask for your input. Do not @ the person who did the assignment otherwise.
  2. Read the CONTRIBUTING.md file make

Copy link

codecov bot commented Nov 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.28%. Comparing base (3f6b41a) to head (3da8ce2).
Report is 1 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2412      +/-   ##
===========================================
- Coverage    98.28%   98.28%   -0.01%     
===========================================
  Files          283      283              
  Lines         8229     8221       -8     
  Branches      2352     2383      +31     
===========================================
- Hits          8088     8080       -8     
  Misses         132      132              
  Partials         9        9              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (18)
src/screens/UserPortal/UserScreen/UserScreen.tsx (1)

Route to title mapping needs to be updated

The current mapping is incomplete. Based on the routes found in App.tsx and the existing map object in UserScreen.tsx, these routes are missing from the mapping:

  • /user/organizations
  • /user/settings
  • /user/chat
  • /user/event/:orgId/:eventId

The map object is used to determine translation keys based on the second segment of the URL path (location.pathname.split('/')[2]), and missing mappings will result in undefined translation keys.

🔗 Analysis chain

Line range hint 19-27: Verify route to title mapping.

The map object is crucial for determining the correct translation key. Ensure it covers all possible routes and their corresponding title keys.

Let's check for any unmapped routes:


Let me gather more context about the existing route mapping by checking the component code.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all possible routes that might need titles
# Look for route definitions that might need to be included in the map
rg -g '*.tsx' -g '*.ts' "path.*user/[a-zA-Z]+" 

Length of output: 4773


Script:

#!/bin/bash
# Get the full content of UserScreen.tsx to understand the route mapping implementation
cat src/screens/UserPortal/UserScreen/UserScreen.tsx

Length of output: 4204

src/screens/UserPortal/UserScreen/UserScreen.test.tsx (4)

90-90: Consider making the test description more specific.

The test name "renders the correct title based on the titleKey for posts" could be more descriptive about the expected behavior.

-  test('renders the correct title based on the titleKey for posts', () => {
+  test('should render "Posts" title when viewing the posts screen', () => {

Line range hint 94-111: Ensure proper test isolation.

The test modifies the shared mockLocation variable but doesn't reset it afterward, which could affect other tests. Consider using beforeEach/afterEach hooks for proper test isolation.

+beforeEach(() => {
+  mockLocation = '/user/organization/123';
+});

 test('renders the correct title based on the titleKey', () => {
   mockLocation = '/user/people/123';
   // ... rest of the test
 });

Line range hint 112-140: Enhance test maintainability and readability.

Consider these improvements:

  1. Add window size cleanup after test
  2. Use constants for window sizes
  3. Add more descriptive assertion messages
+const MOBILE_WIDTH = 800;
+const DESKTOP_WIDTH = 1000;

 test('LeftDrawer should toggle correctly based on window size and user interaction', async () => {
   // ... setup code ...

-  resizeWindow(800);
+  resizeWindow(MOBILE_WIDTH);
   clickToggleMenuBtn(toggleButton);
-  expect(icon).toHaveClass('fa fa-angle-double-left');
+  expect(icon).toHaveClass('fa fa-angle-double-left', 'Icon should indicate collapsed state on mobile');

-  resizeWindow(1000);
+  resizeWindow(DESKTOP_WIDTH);
   // ... rest of the test ...

+  // Cleanup
+  resizeWindow(window.innerWidth);
 });

Line range hint 141-156: Fix critical issues in navigation test implementation.

The test has several issues:

  1. The useNavigate mock implementation is incorrect
  2. The assertion should check the navigate function call instead of window.location
  3. Missing cleanup of mockID
+beforeEach(() => {
+  mockID = '123';
+});

 test('should be redirected to root when orgId is undefined', async () => {
   mockID = undefined;
   const navigate = jest.fn();
-  jest.spyOn({ useNavigate }, 'useNavigate').mockReturnValue(navigate);
+  (useNavigate as jest.Mock).mockReturnValue(navigate);
   
   render(
     // ... component render ...
   );
-  expect(window.location.pathname).toEqual('/');
+  expect(navigate).toHaveBeenCalledWith('/');
 });
package.json (1)

11-12: Consider pinning exact versions.

Using caret (^) for version ranges can lead to unexpected behavior if patches include breaking changes. Consider pinning exact versions for more predictable builds:

-    "@dicebear/collection": "^9.2.2",
-    "@dicebear/core": "^9.2.2",
+    "@dicebear/collection": "9.2.2",
+    "@dicebear/core": "9.2.2",
src/components/UserPortal/CreateDirectChat/CreateDirectChat.tsx (4)

64-66: Remove commented-out code.

These commented-out lines should be removed if the translation functionality is no longer needed.


Line range hint 75-89: Add error handling and loading state for chat creation.

The handleCreateDirectChat function should:

  1. Handle potential errors from the mutation
  2. Show a loading state during creation
  3. Provide feedback on success/failure

Consider updating the implementation:

 const handleCreateDirectChat = async (id: string): Promise<void> => {
+  try {
+    setIsCreating(true);
     await createChat({
       variables: {
         organizationId,
         userIds: [userId, id],
         isGroup: false,
       },
     });
     await chatsListRefetch();
     toggleCreateDirectChatModal();
+    // Show success message
+  } catch (error) {
+    // Show error message
+  } finally {
+    setIsCreating(false);
+  }
 };

Line range hint 98-112: Improve search functionality implementation.

The current search implementation has limitations:

  1. Splitting by space assumes a perfect "firstName lastName" format
  2. Doesn't handle multiple spaces or middle names
  3. Performs exact matches which might not be user-friendly

Consider using a single search field that searches across both first and last names, or implement a more robust name parsing logic.


Line range hint 116-190: Consider consolidating UI libraries.

The component mixes Material-UI and React-Bootstrap components, which can lead to:

  1. Inconsistent styling and behavior
  2. Increased bundle size
  3. Maintenance complexity

Consider standardizing on one UI library throughout the application for better consistency and maintainability.

src/screens/UserPortal/Posts/Posts.tsx (1)

211-229: Consider enhancing type safety and readability.

While the null handling is good, we can improve the code further:

Consider this refactoring for better type safety and readability:

-    const allLikes: InterfacePostLikes = likedBy.map((value) => ({
-      firstName: value.firstName,
-      lastName: value.lastName,
-      id: value._id,
-    }));
+    const allLikes = likedBy.map((value): InterfacePostLikes[number] => ({
+      firstName: value.firstName,
+      lastName: value.lastName,
+      id: value._id,
+    }));

-    const postComments: InterfacePostComments =
-      comments?.map((value) => ({
-        id: value.id,
-        creator: {
-          firstName: value.creator?.firstName ?? '',
-          lastName: value.creator?.lastName ?? '',
-          id: value.creator?.id ?? '',
-          email: value.creator?.email ?? '',
-        },
-        likeCount: value.likeCount,
-        likedBy: value.likedBy?.map((like) => ({ id: like?.id ?? '' })) ?? [],
-        text: value.text,
-      })) ?? [];
+    const postComments = comments?.map((value): InterfacePostComments[number] => {
+      const creator = value.creator ?? {};
+      return {
+        id: value.id,
+        creator: {
+          firstName: creator.firstName ?? '',
+          lastName: creator.lastName ?? '',
+          id: creator.id ?? '',
+          email: creator.email ?? '',
+        },
+        likeCount: value.likeCount,
+        likedBy: value.likedBy?.map((like) => ({ id: like?.id ?? '' })) ?? [],
+        text: value.text,
+      };
+    }) ?? [];

Changes:

  1. Added explicit type annotations in map callbacks
  2. Destructured creator to reduce nesting
  3. Improved code structure for better readability
src/components/UserPortal/UserSidebar/UserSidebar.test.tsx (1)

3-3: Consider keeping the waitFor utility.

While removing waitFor in favor of a custom wait function works, the @testing-library/react's waitFor utility is specifically designed for testing async operations and provides better error messages and debugging capabilities.

-import { fireEvent, render, screen } from '@testing-library/react';
+import { fireEvent, render, screen, waitFor } from '@testing-library/react';
schema.graphql (3)

1095-1095: Consider adding filtering options to venue query.

While the basic query structure is correct, consider adding filtering capabilities similar to other queries in the schema (e.g., where input type) to make it more consistent with the rest of the API.

-  venue(id: ID!): [Venue]
+  venue(
+    id: ID!
+    where: VenueWhereInput
+  ): [Venue]

Consider adding this input type:

input VenueWhereInput {
  name_contains: String
  capacity_gte: Int
  capacity_lte: Int
}

1544-1544: Consider adding validation constraints for chat creation.

While the chatInput type contains the necessary fields, consider adding validation constraints:

  1. For group chats, name should be required
  2. Minimum number of users for group/direct chats
input chatInput {
  isGroup: Boolean!
-  organizationId: ID
+  organizationId: ID!
  userIds: [ID!]!
-  name: String
+  name: String @constraint(
+    minLength: 1
+    maxLength: 50
+    required: true
+    requiredIf: "isGroup"
+  )
}

Line range hint 1-1544: Consider implementing consistent pagination pattern across queries.

Some queries use offset-based pagination (skip/first), while others use cursor-based pagination (after/before/first/last). Consider standardizing the pagination approach across all connection types for consistency.

src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx (2)

Line range hint 832-838: Consider enhancing the wait utility function

The wait utility function could be improved for better maintainability and reliability:

  1. Add JSDoc documentation
  2. Consider increasing the default timeout
  3. Add type safety for the return value
+/**
+ * Utility function to wait for async operations in tests
+ * @param ms - The number of milliseconds to wait (default: 500)
+ * @returns Promise<void>
+ */
-async function wait(ms = 100): Promise<void> {
+async function wait(ms = 500): Promise<void> {
   await act(() => {
     return new Promise((resolve) => {
       setTimeout(resolve, ms);
     });
   });
 }

Line range hint 840-957: Enhance test coverage for edge cases

The current test suite covers basic functionality but would benefit from additional test cases:

  1. Error handling scenarios (network errors, validation errors)
  2. User search functionality validation
  3. Empty states and loading states
  4. Edge cases for user selection

Here's an example of additional test cases to consider:

test('should handle network error when creating chat', async () => {
  // Mock implementation with network error
});

test('should handle empty user search results', async () => {
  // Test empty state rendering
});

test('should show loading state while searching users', async () => {
  // Test loading indicator
});

test('should validate minimum/maximum user selection', async () => {
  // Test user selection constraints
});
src/screens/UserPortal/Chat/Chat.test.tsx (1)

Line range hint 1588-1677: Consider enhancing test coverage for group chat and sidebar.

While the basic functionality is tested, consider adding these scenarios:

  1. Group chat creation with multiple users
  2. Form validation for group name
  3. Sidebar state persistence across renders
  4. Edge cases for different screen sizes

Here's a suggested addition to improve the group chat test:

 test('create new group chat', async () => {
   // ... existing setup ...
   
   fireEvent.click(newGroupChatBtn);
+  // Test form validation
+  const submitButton = screen.getByTestId('submitBtn');
+  fireEvent.click(submitButton);
+  expect(screen.getByText(/group name is required/i)).toBeInTheDocument();
+  
+  // Test adding multiple users
+  const userInput = screen.getByTestId('userSelect');
+  fireEvent.change(userInput, { target: { value: 'Test User' } });
+  const userOption = await screen.findByText('Test User');
+  fireEvent.click(userOption);
+  
+  // Test group creation
+  const groupNameInput = screen.getByTestId('groupName');
+  fireEvent.change(groupNameInput, { target: { value: 'New Group' } });
+  fireEvent.click(submitButton);
+  await waitFor(() => {
+    expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
+  });

   // ... existing cleanup ...
 });
🛑 Comments failed to post (2)
package.json (1)

11-12: 💡 Codebase verification

Breaking changes detected in dicebear upgrade

The upgrade from v8 to v9 of @dicebear packages requires attention:

  • Found active usage in src/components/Avatar/Avatar.tsx which imports both @dicebear/core and @dicebear/collection
  • Multiple test files reference dicebear URLs that may need updates (currently using v5.x API endpoints)
  • Git history shows multiple major version jumps (v7 → v8 → v9) with breaking changes

Please ensure:

  1. Update API endpoint URLs in tests from api.dicebear.com/5.x to the current version
  2. Review dicebear v9 migration guide for breaking changes
  3. Update avatar implementation in Avatar.tsx according to v9 API changes
🔗 Analysis chain

Major version upgrade requires careful review.

The upgrade from v8 to v9 of @dicebear packages represents a major version change that may include breaking changes.

Let's verify the changelog and potential breaking changes:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for breaking changes in @dicebear packages

# Test 1: Look for package.json files that might be using these dependencies
echo "Searching for other package.json files using @dicebear..."
fd package.json --exec grep -l "@dicebear"

# Test 2: Look for actual usage of @dicebear in the codebase
echo "Checking usage of @dicebear in the codebase..."
rg "@dicebear" --type ts --type tsx

Length of output: 349


Script:

#!/bin/bash
# Let's try a different approach to find @dicebear usage

# Search for any files containing dicebear (case-insensitive)
echo "Searching for files containing dicebear..."
rg -i "dicebear" --type js --type jsx --type ts

# Look for imports/requires of dicebear packages
echo "Searching for specific dicebear imports..."
ast-grep --pattern 'import $_ from "@dicebear/$_"'

# Check package-lock.json for current version
echo "Checking package-lock.json for version info..."
rg "\"@dicebear/" package-lock.json -A 3

Length of output: 4907


Script:

#!/bin/bash
# Let's search for files with common extensions that might use dicebear
echo "Searching for files containing dicebear in source files..."
rg -i "dicebear" -g "*.{js,jsx,ts,tsx,vue}"

# Look for any avatar-related components that might use dicebear
echo "Searching for avatar-related files..."
fd -e js -e jsx -e ts -e tsx -e vue -g "*avatar*" --exec grep -l -i "dicebear" {} \;

# Check for breaking changes by examining git history
echo "Checking git history for previous dicebear version..."
git log -p package.json | grep -A 2 -B 2 "@dicebear"

Length of output: 10863

src/components/UserPortal/CreateDirectChat/CreateDirectChat.tsx (1)

27-31: 💡 Codebase verification

⚠️ Potential issue

The return type ApolloQueryResult<string> is incorrect and should be updated

The chatsListRefetch function is used to refresh the chat list after creating a chat, but its return type is incorrectly specified. Based on the USERS_CONNECTION_LIST query definition, it should return a complex object containing user data with firstName and lastName fields, not a string. Update the return type to match the actual query result structure:

Promise<ApolloQueryResult<{
  users: {
    user: {
      firstName: string;
      lastName: string;
    }
  }
}>>
🔗 Analysis chain

Review the Apollo query result type.

While removing any types improves type safety, the return type Promise<ApolloQueryResult<string>> seems incorrect. Apollo queries typically return complex objects rather than primitive strings. Consider using the actual query result type or a more appropriate interface.

Let's verify the actual query result type:


Let's gather more information about the query definition and its usage:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for the query definition to understand the expected return type
rg -A 5 "USERS_CONNECTION_LIST" --type ts

Length of output: 10922


Script:

#!/bin/bash
# Get the full query definition and its return type
rg -A 15 "export const USERS_CONNECTION_LIST = gql" --type ts

# Find where chatsListRefetch is used
ast-grep --pattern 'chatsListRefetch($$$)'

Length of output: 1163

schema.graphql Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (3)
schema.graphql (3)

208-225: LGTM! Well-structured chat type definitions.

The new chat types are well-designed with proper separation between direct and group chats. The types include all necessary fields and maintain consistency in timestamps and IDs.

Consider adding these fields to enhance functionality:

  • lastMessage field in both chat types for quick preview
  • isTyping field for real-time typing indicators
  • readBy field in messages to track message read status

Also applies to: 464-482


1558-1567: Add input validation for chat creation.

The input types are well-structured but lack validation constraints.

Consider adding these validations:

  • Minimum/maximum length for group chat title
  • Minimum number of users (e.g., at least 2 for direct chat)
  • Maximum number of users for group chats

Example:

input createGroupChatInput {
  organizationId: ID!
  title: String! @constraint(minLength: 1, maxLength: 100)
  userIds: [ID!]! @constraint(minItems: 2, maxItems: 100)
}

732-739: Add support for file attachments in chat messages.

The message sending mutations currently only support text content.

Consider adding file attachment support:

input ChatMessageInput {
  messageContent: String!
  fileAttachments: [Upload]
}

type Mutation {
  sendMessageToDirectChat(
    chatId: ID!
    message: ChatMessageInput!
  ): DirectChatMessage!
}
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 31f3d00 and 9715b32.

📒 Files selected for processing (2)
  • schema.graphql (14 hunks)
  • src/components/UserPortal/CreateDirectChat/CreateDirectChat.tsx (1 hunks)
🔇 Additional comments (2)
src/components/UserPortal/CreateDirectChat/CreateDirectChat.tsx (1)

27-30: 🛠️ Refactor suggestion

Verify the Apollo query result type.

While changing id: any to id: string improves type safety, the return type ApolloQueryResult<string> seems incorrect. Apollo queries typically return complex objects rather than primitive types.

Let's verify the actual query response structure:

Consider using a more specific type that matches your GraphQL schema, for example:

Promise<ApolloQueryResult<{
  chats: {
    id: string;
    // other chat fields
  }[];
}>>
schema.graphql (1)

1162-1164: LGTM! Well-implemented real-time message subscriptions.

The subscription types are properly defined for both direct and group chat messages, enabling real-time updates.

schema.graphql Outdated Show resolved Hide resolved
schema.graphql Outdated Show resolved Hide resolved
coderabbitai[bot]
coderabbitai bot previously approved these changes Nov 7, 2024
schema.graphql Outdated Show resolved Hide resolved
@VanshikaSabharwal
Copy link
Contributor Author

I reverted the changes from schema.graphql file . Actually when i was clicking save due to prettier the commas were getting removed. So, this was causing the issue. Do i have to revert changes from other files as well ? because the test is still failing even after reverting changes from schema.graphql. see https://github.com/PalisadoesFoundation/talawa-admin/actions/runs/11721560114/job/32649240787?pr=2412#logs

@pranshugupta54
Copy link
Member

Please increase the code coverage

@VanshikaSabharwal
Copy link
Contributor Author

Please increase the code coverage

Okay, doing it

@VanshikaSabharwal
Copy link
Contributor Author

I have increased the code coverage .

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
src/screens/UserPortal/Posts/Posts.tsx (1)

211-228: Consider using object shorthand notation for cleaner code.

The data transformation logic is correct and handles edge cases well. However, we can make it more concise using object shorthand notation where property names match variable names.

Consider this refactor for improved readability:

  const allLikes: InterfacePostLikes = likedBy.map(({ firstName, lastName, _id }) => ({
    firstName,
    lastName,
    id: _id,
  }));

  const postComments: InterfacePostComments = comments?.map(({
    id,
    creator,
    likeCount,
    likedBy,
    text
  }) => ({
    id,
    creator: {
      firstName: creator?.firstName ?? '',
      lastName: creator?.lastName ?? '',
      id: creator?.id ?? '',
      email: creator?.email ?? '',
    },
    likeCount,
    likedBy: likedBy?.map(like => ({ id: like?.id ?? '' })) ?? [],
    text,
  }));
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 250beb0 and 3da8ce2.

📒 Files selected for processing (1)
  • src/screens/UserPortal/Posts/Posts.tsx (2 hunks)
🔇 Additional comments (2)
src/screens/UserPortal/Posts/Posts.tsx (2)

57-64: LGTM: Type definition changes look good.

The standardization of ID field names from _id to id improves consistency in the codebase.


Line range hint 279-281: Verify title alignment fix.

While the code structure looks correct, please ensure that the alignment fix mentioned in PR #2071 is working as expected across different screen sizes.

✅ Verification successful

Let me check the specific CSS module for the Posts component and any related changes.


Based on the CSS changes history and content, I can now provide a final response:

Title alignment has been properly implemented

The code shows that the heading styles have been properly defined with consistent sizing (1.1rem) and the post container has appropriate spacing with proper margins and padding. Recent changes in PR #1896 include a complete redesign of the Posts screen that properly addresses title alignment through:

  • Consistent font size for headings
  • Proper container spacing with 1rem padding
  • Clear margin structure (0.5rem top, 1rem bottom) for the post input container
  • Responsive layout with proper width constraints
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for related CSS changes that might affect title alignment

# Look for CSS module changes
rg -l "title|heading" --type css

Length of output: 2544


Script:

#!/bin/bash
# Check the Posts.module.css file content for heading styles
cat src/screens/UserPortal/Posts/Posts.module.css

# Check git history for recent changes to this CSS file
git log -p src/screens/UserPortal/Posts/Posts.module.css

Length of output: 8893

@VanshikaSabharwal
Copy link
Contributor Author

Can I raise another PR for issue #2402, as it is assigned to me? I just want to confirm that it won't cause any conflicts with the next PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants